meson: build libnvme directly instead of as a subproject#2967
meson: build libnvme directly instead of as a subproject#2967igaw merged 2 commits intolinux-nvme:masterfrom
Conversation
595f21a to
363f4d9
Compare
|
Thanks for this work! There is some nice cleanups and improvements 👍 A few things should be addressed before the merge though. Nothing too serious IMO but the last point:
FAILED: [code=1] libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o
cc -Ilibnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p -Ilibnvme/libnvme -I../libnvme/libnvme -I. -I.. -Iccan -I../ccan -Ilibnvme -I../libnvme -Ilibnvme/src -I../libnvme/src -I/usr/include/json-c -I/usr/include/python3.14 -fvisibility=hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=gnu99 -O2 -g -fomit-frame-pointer -D_GNU_SOURCE -include project-config.h -fPIC -MD -MQ libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o -MF libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o.d -o libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o -c libnvme/libnvme/nvme_wrap.c
In file included from /usr/include/python3.14/Python.h:71,
from libnvme/libnvme/nvme_wrap.c:203:
/usr/include/python3.14/pyport.h:660:35: error: missing ‘)’ after ‘__has_attribute’
660 | #if _Py__has_attribute(fallthrough) && (!defined(__clang__) || \
| ^
./project-config.h:86:35: error: missing binary operator before token ‘(’
86 | #define fallthrough __attribute__((__fallthrough__))
| ^
/usr/include/python3.14/pyport.h:25:49: note: in definition of macro ‘_Py__has_attribute’
25 | # define _Py__has_attribute(x) __has_attribute(x)
| ^
/usr/include/python3.14/pyport.h:660:24: note: in expansion of macro ‘fallthrough’
660 | #if _Py__has_attribute(fallthrough) && (!defined(__clang__) || \
| ^~~~~~~~~~~
› meson install -C .build-incl-lib
ninja: Entering directory `/home/wagi/work/nvme-cli/.build-incl-lib'
ninja: no work to do.
Installing libnvme/src/libnvme.so.3.0.0 to /tmp/test/lib64
Installing nvme to /tmp/test/sbin
Installing /home/wagi/work/nvme-cli/libnvme/src/libnvme.h to /tmp/test/include
'/tmp/test/include/libnvme.h': Unable to set owner 0 and group 0: Operation not permitted, ignoring...
Installing /home/wagi/work/nvme-cli/libnvme/src/libnvme-mi.h to /tmp/test/include
To your questions:
› ./scripts/build.sh -m muon
~/work/nvme-cli/.build-tools/muon ~/work/nvme-cli |
|
Good feedback. I'll start working on it. Thanks. |
363f4d9 to
af2cbd6
Compare
|
I fixed the following:
I have a few questions regarding the following items:
|
6568ff0 to
8b3fd95
Compare
|
@igaw - Added 2 new meson options: Regarding |
a651347 to
de94af4
Compare
|
@igaw - Regarding your earlier comment:
Building the library ( Please let me know how you want to proceed. |
e395c87 to
961c72c
Compare
How do you build nvme-cli here? When building the project separately, the library needs to be build AND installed first., basically what |
Sorry, I misunderstood. When you said "This means also we should be able to build nvme-cli without library" I thought you meant that we should be able to build nvme-cli w/o libnvme present. We are on the same page. This patch will allow building nvme-cli by itself as long as libnvme has previously been built and installed. |
$ meson setup .build --debug --optimization=0 --reconfigure -Dlibnvme=enabled -Dnvme=disabled --prefix=/tmp/test
[...]
$ meson test -C .build
ninja: Entering directory `/home/wagi/work/nvme-cli-upstream/.build'
[113/113] Linking target libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so
1/47 libnvme - python-import-libnvme OK 0.06s
2/47 libnvme - python-create-ctrl-object OK 0.06s
3/47 libnvme - python-sigsegv-during-gc OK 0.06s
4/47 libnvme - cpp-dump OK 0.05s
5/47 libnvme - cpp-misc OK 0.05s
6/47 libnvme - mi OK 0.04s
7/47 libnvme - uuid OK 0.03s
8/47 libnvme - uriparser OK 0.03s
9/47 libnvme - util OK 0.02s
10/47 libnvme - psk OK 0.02s
11/47 libnvme - ana OK 0.02s
12/47 libnvme - discovery OK 0.01s
13/47 libnvme - features OK 0.01s
14/47 libnvme - mi-mctp OK 0.08s
15/47 libnvme - python-read-nbft-file OK 0.10s
16/47 libnvme - tree OK 0.08s
17/47 libnvme - identify OK 0.05s
18/47 libnvme - logs OK 0.05s
19/47 libnvme - zns OK 0.05s
20/47 libnvme - misc OK 0.04s
21/47 libnvme - NBFT-auto-ipv6 OK 0.04s
22/47 libnvme - NBFT-dhcp-ipv6 OK 0.04s
23/47 libnvme - NBFT-rhpoc OK 0.04s
24/47 libnvme - NBFT-static-ipv4 OK 0.03s
25/47 libnvme - NBFT-static-ipv4-discovery OK 0.03s
26/47 libnvme - NBFT-static-ipv6 OK 0.03s
27/47 libnvme - NBFT-Dell.PowerEdge.R760 OK 0.03s
28/47 libnvme - NBFT-Dell.PowerEdge.R660-fw1.5.5-single OK 0.02s
29/47 libnvme - NBFT-Dell.PowerEdge.R660-fw1.5.5-mpath+discovery OK 0.07s
30/47 libnvme - NBFT-mpath+disc-ipv4+6_half OK 0.07s
31/47 libnvme - NBFT-ipv6-noip+disc OK 0.07s
32/47 libnvme - NBFT-empty OK 0.07s
33/47 libvnme - psk-json-tls_key-1 OK 0.05s
34/47 libvnme - psk-json-tls_key-2 OK 0.04s
35/47 nvme-cli - uint128 OK 0.03s
36/47 nvme-cli - suffix_si_parse OK 0.03s
37/47 libnvme - NBFT-bad-oldspec EXPECTEDFAIL 0.07s exit status 2
38/47 libnvme - NBFT-random-noise EXPECTEDFAIL 0.07s exit status 2
39/47 nvme-cli - suffix_binary_parse OK 0.03s
40/47 libnvme - config-pcie-with-tcp-config OK 0.06s
41/47 nvme-cli - uint128-si OK 0.02s
42/47 nvme-cli - argconfig_parse OK 0.01s
43/47 libnvme - hostnqn-order OK 0.06s
44/47 libnvme - config-pcie OK 0.07s
45/47 libnvme - tree-apple-nvme OK 0.08s
46/47 libnvme - tree-pcie OK 0.10s
47/47 libnvme - kdoc OK 0.29s
|
961c72c to
37c5038
Compare
|
@igaw - I just pushed fixes for the following two issues (good catch BTW).
I'll be looking at |
|
There are several places in Question: Do we want to treat warnings as errors or not? And BTW, meson complains when we use both |
|
When the project were complete independent repos, I didn't want that building nvme-cli would break on a warning from libnvme. But since both project are now inside the same repo I'd say we an do werror for nvme-cli and libnvme. |
37c5038 to
4217871
Compare
|
The There are a couple of things that are not working, but I don't think they're related to my changes:
|
|
FWIW: I've added a section in the README, explaining how to use the existing containers to build cross: |
Could you also rebase it to the latest master. I am sure distros are able to build several packages from on source, I'd want to keep the builds and installs currently separately. |
1611d1c to
f377a1e
Compare
|
@igaw - I rebased with latest master and fixed the docs to honor Some of the GitHub Actions are failing. I'm investigating... |
|
A lot of the Actions are failing because GitHub is invoking the |
3d430f6 to
5d28dba
Compare
Change the build flow so that libnvme is no longer defined as a Meson subproject of nvme-cli. Instead, include libnvme's meson.build directly from the top-level meson.build. This allows sharing a common ccan directory between nvme-cli and libnvme, and generates a single configuration header, project-config.h (formerly config.h), used by all components. The old file name conflicted with plugins/solidigm/solidigm-telemetry/config.h, hence the rename. Also reformat all meson.build files to fix indentation inconsistencies and other cosmetic discrepancies. Signed-off-by: Martin Belanger <[email protected]>
5d28dba to
3c46520
Compare
Thanks for the info. I tried it and it works. The only problem is that all the NBFT tests are failing because the utility |
For now, the library should be buildable independent of the nvme-cli binary. Add a new target into the build script and update the workflow accordingly. This only updates the build part, there is more work to be done for the other workflows etc. Signed-off-by: Daniel Wagner <[email protected]>
c2b0fea to
f42a92c
Compare
This is likely caused by a missing hooks in the kernel for executing cross compiled binaries. That is the hook is missing in |
|
It was a big task :) Let's go with this and improve the missing bits over time. Thanks a lot! |
Change the build flow so that libnvme is no longer defined as a Meson subproject of nvme-cli. Instead, include libnvme's meson.build directly from the top-level meson.build.
This allows sharing a common ccan directory between nvme-cli and libnvme, and generates a single configuration header, project-config.h (formerly config.h), used by all components. The old file name conflicted with plugins/solidigm/solidigm-telemetry/config.h, hence the rename.
Also reformat all meson.build files to fix indentation inconsistencies and other cosmetic discrepancies.
@igaw - I tested different build options (enabling all possible options including generating the docs). Everything is working although I couldn't test cross-compilation and other exotic options (e.g. muon).
One thing that I wasn't sure about is the licensing which is slightly different between nvme-cli and libnvme. I have kept both licenses as follows. I made sure these would get used in the
nvme.spec.inandlibnvme.spec.in.